From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Fri, 12 Jan 2018 18:08:59 +0000 (+0000)
Subject: Merge "Replace cpPosTime cookie/parameter with cpPosIndex"
X-Git-Tag: 1.31.0-rc.0~923
X-Git-Url: http://git.cyclocoop.org/%28%5B%5E/404?a=commitdiff_plain;h=92f9cb055173058c7c09ea746ba75109fbe0f7ec;hp=-c;p=lhc%2Fweb%2Fwiklou.git

Merge "Replace cpPosTime cookie/parameter with cpPosIndex"
---

92f9cb055173058c7c09ea746ba75109fbe0f7ec
diff --combined includes/MediaWiki.php
index a217cd1418,b1b1a7aee7..150c72f804
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@@ -603,50 -603,54 +603,54 @@@ class MediaWiki 
  		DeferredUpdates::doUpdates( 'enqueue', DeferredUpdates::PRESEND );
  		wfDebug( __METHOD__ . ': pre-send deferred updates completed' );
  
- 		// Decide when clients block on ChronologyProtector DB position writes
- 		$urlDomainDistance = (
- 			$request->wasPosted() &&
- 			$output->getRedirect() &&
- 			$lbFactory->hasOrMadeRecentMasterChanges( INF )
- 		) ? self::getUrlDomainDistance( $output->getRedirect() ) : false;
+ 		// Should the client return, their request should observe the new ChronologyProtector
+ 		// DB positions. This request might be on a foreign wiki domain, so synchronously update
+ 		// the DB positions in all datacenters to be safe. If this output is not a redirect,
+ 		// then OutputPage::output() will be relatively slow, meaning that running it in
+ 		// $postCommitWork should help mask the latency of those updates.
+ 		$flags = $lbFactory::SHUTDOWN_CHRONPROT_SYNC;
+ 		$strategy = 'cookie+sync';
  
  		$allowHeaders = !( $output->isDisabled() || headers_sent() );
- 		if ( $urlDomainDistance === 'local' || $urlDomainDistance === 'remote' ) {
- 			// OutputPage::output() will be fast; $postCommitWork will not be useful for
- 			// masking the latency of syncing DB positions accross all datacenters synchronously.
- 			// Instead, make use of the RTT time of the client follow redirects.
- 			$flags = $lbFactory::SHUTDOWN_CHRONPROT_ASYNC;
- 			$cpPosTime = microtime( true );
- 			// Client's next request should see 1+ positions with this DBMasterPos::asOf() time
- 			if ( $urlDomainDistance === 'local' && $allowHeaders ) {
- 				// Client will stay on this domain, so set an unobtrusive cookie
- 				$expires = time() + ChronologyProtector::POSITION_TTL;
- 				$options = [ 'prefix' => '' ];
- 				$request->response()->setCookie( 'cpPosTime', $cpPosTime, $expires, $options );
- 			} else {
- 				// Cookies may not work across wiki domains, so use a URL parameter
- 				$safeUrl = $lbFactory->appendPreShutdownTimeAsQuery(
- 					$output->getRedirect(),
- 					$cpPosTime
- 				);
- 				$output->redirect( $safeUrl );
+ 		if ( $output->getRedirect() && $lbFactory->hasOrMadeRecentMasterChanges( INF ) ) {
+ 			// OutputPage::output() will be fast, so $postCommitWork is useless for masking
+ 			// the latency of synchronously updating the DB positions in all datacenters.
+ 			// Try to make use of the time the client spends following redirects instead.
+ 			$domainDistance = self::getUrlDomainDistance( $output->getRedirect() );
+ 			if ( $domainDistance === 'local' && $allowHeaders ) {
+ 				$flags = $lbFactory::SHUTDOWN_CHRONPROT_ASYNC;
+ 				$strategy = 'cookie'; // use same-domain cookie and keep the URL uncluttered
+ 			} elseif ( $domainDistance === 'remote' ) {
+ 				$flags = $lbFactory::SHUTDOWN_CHRONPROT_ASYNC;
+ 				$strategy = 'cookie+url'; // cross-domain cookie might not work
  			}
- 		} else {
- 			// OutputPage::output() is fairly slow; run it in $postCommitWork to mask
- 			// the latency of syncing DB positions accross all datacenters synchronously
- 			$flags = $lbFactory::SHUTDOWN_CHRONPROT_SYNC;
- 			if ( $lbFactory->hasOrMadeRecentMasterChanges( INF ) && $allowHeaders ) {
- 				$cpPosTime = microtime( true );
- 				// Set a cookie in case the DB position store cannot sync accross datacenters.
- 				// This will at least cover the common case of the user staying on the domain.
+ 		}
+ 
+ 		// Record ChronologyProtector positions for DBs affected in this request at this point
+ 		$cpIndex = null;
+ 		$lbFactory->shutdown( $flags, $postCommitWork, $cpIndex );
+ 		wfDebug( __METHOD__ . ': LBFactory shutdown completed' );
+ 
+ 		if ( $cpIndex > 0 ) {
+ 			if ( $allowHeaders ) {
  				$expires = time() + ChronologyProtector::POSITION_TTL;
  				$options = [ 'prefix' => '' ];
- 				$request->response()->setCookie( 'cpPosTime', $cpPosTime, $expires, $options );
+ 				$request->response()->setCookie( 'cpPosIndex', $cpIndex, $expires, $options );
+ 			}
+ 
+ 			if ( $strategy === 'cookie+url' ) {
+ 				if ( $output->getRedirect() ) { // sanity
+ 					$safeUrl = $lbFactory->appendShutdownCPIndexAsQuery(
+ 						$output->getRedirect(),
+ 						$cpIndex
+ 					);
+ 					$output->redirect( $safeUrl );
+ 				} else {
+ 					$e = new LogicException( "No redirect; cannot append cpPosIndex parameter." );
+ 					MWExceptionHandler::logException( $e );
+ 				}
  			}
  		}
- 		// Record ChronologyProtector positions for DBs affected in this request at this point
- 		$lbFactory->shutdown( $flags, $postCommitWork );
- 		wfDebug( __METHOD__ . ': LBFactory shutdown completed' );
  
  		// Set a cookie to tell all CDN edge nodes to "stick" the user to the DC that handles this
  		// POST request (e.g. the "master" data center). Also have the user briefly bypass CDN so
@@@ -727,12 -731,10 +731,12 @@@
  		if ( function_exists( 'register_postsend_function' ) ) {
  			// https://github.com/facebook/hhvm/issues/1230
  			register_postsend_function( $callback );
 +			/** @noinspection PhpUnusedLocalVariableInspection */
  			$blocksHttpClient = false;
  		} else {
  			if ( function_exists( 'fastcgi_finish_request' ) ) {
  				fastcgi_finish_request();
 +				/** @noinspection PhpUnusedLocalVariableInspection */
  				$blocksHttpClient = false;
  			} else {
  				// Either all DB and deferred updates should happen or none.